Skip to content

GH-49470: [C++][Gandiva] Fix crashes in substring_index and truncate with extreme integer values#49471

Merged
zanmato1984 merged 2 commits intoapache:mainfrom
dmitry-chirkov-dremio:fix-gandiva-extreme-int-crashes
Mar 15, 2026
Merged

GH-49470: [C++][Gandiva] Fix crashes in substring_index and truncate with extreme integer values#49471
zanmato1984 merged 2 commits intoapache:mainfrom
dmitry-chirkov-dremio:fix-gandiva-extreme-int-crashes

Conversation

@dmitry-chirkov-dremio
Copy link
Contributor

@dmitry-chirkov-dremio dmitry-chirkov-dremio commented Mar 9, 2026

Rationale for this change

Two Gandiva functions crash when called with extreme integer parameter values:

  1. substring_index(VARCHAR, VARCHAR, INT) crashes with SIGBUS when count is INT_MIN
  2. truncate(BIGINT, INT) crashes with SIGSEGV when scale is INT_MAX or INT_MIN

What changes are included in this PR?

substring_index fix (gdv_string_function_stubs.cc):

  • Replace abs(cnt) with safe int64_t computation to avoid undefined behavior when cnt == INT_MIN

truncate fix (precompiled/extended_math_ops.cc):

  • Return input unchanged for positive scales (no-op for integers)
  • Return 0 for scales < -38 to prevent out-of-bounds access in GetScaleMultiplier

Are these changes tested?

Yes. Added coverage for INT_MAX/INT_MIN values in gdv_function_stubs_test.cc and extended_math_ops_test.cc.

Are there any user-facing changes?

No.

This PR contains a "Critical Fix". These changes fix crashes caused by:

  • abs(INT_MIN) triggering undefined behavior (integer overflow) in substring_index
  • Out-of-bounds array access in GetScaleMultiplier when truncate receives extreme scale values

@github-actions
Copy link

github-actions bot commented Mar 9, 2026

⚠️ GitHub issue #49470 has been automatically assigned in GitHub to PR creator.

@github-actions github-actions bot added awaiting committer review Awaiting committer review and removed awaiting review Awaiting review labels Mar 9, 2026
Copy link
Contributor

@zanmato1984 zanmato1984 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The failing CI seems unrelated .

+1

@zanmato1984 zanmato1984 merged commit d6ce56e into apache:main Mar 15, 2026
53 of 54 checks passed
@zanmato1984 zanmato1984 removed the awaiting committer review Awaiting committer review label Mar 15, 2026
@conbench-apache-arrow
Copy link

After merging your PR, Conbench analyzed the 3 benchmarking runs that have been run so far on merge-commit d6ce56e.

There were no benchmark performance regressions. 🎉

The full Conbench report has more details. It also includes information about 1 possible false positive for unstable benchmarks that are known to sometimes produce them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants